[refactor] 북마크 API DB 쿼리 최적화, 인덱스 개선 및 perf-analyzer 에이전트 추가#353
Merged
[refactor] 북마크 API DB 쿼리 최적화, 인덱스 개선 및 perf-analyzer 에이전트 추가#353
Conversation
- findPlacesWithoutTags: loadPlaceTagsAndTags 제거 → LEFT JOIN FETCH 통합 (2쿼리 → 1쿼리) - findPlacesWithTags: LEFT JOIN FETCH 추가로 PlaceTag N+1 방지 - PlaceService: getBookmarkedPlaceIdsForTown 중복 호출 제거, orderedIds 재사용으로 북마크 최신순 정렬 유지
- bookmarks: (user_id, created_at) → (user_id, target_type, created_at DESC) - target_type 필터링 시 인덱스 풀 스캔 제거 - places: (town_id) → (town_id, active, created_at DESC) - active 필터 + created_at 정렬을 인덱스 내에서 처리 - 엔티티 @Index 어노테이션 동기화
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough북마크 및 장소 조회 API의 데이터베이스 부하를 개선하기 위해 쿼리 최적화와 인덱스 재구성을 수행했습니다. LEFT JOIN FETCH로 N+1 쿼리를 제거하고, 복합 인덱스를 추가하며, 북마크 검색 로직을 개선했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/org/sopt/solply_server/domain/place/service/PlaceService.java (1)
113-134: 회피 가능한 북마크 조회가 아직 두 경로에 남아 있습니다.Line 117-119는
getPlacesByCondition()내부의 태그 검증보다 먼저 실행돼서, 잘못된 태그 조합 요청도getBookmarkedPlaceIdsForTown()을 한 번 타게 됩니다. 그리고 non-bookmark 조회에서places가 비어도 Line 128-130이 다시 북마크 ID를 읽습니다. 이 PR의 목적이 중복 Redis/DB 호출 축소라면 두 경로 모두 먼저 short-circuit하는 편이 낫습니다.♻️ 제안 수정안
+ if (mainTagId != null) { + tagValidator.validatePlaceTagConditions(mainTagId, subTagAIdList, subTagBIdList); + } + // isBookmarkSearch=true 시 orderedIds를 한 번만 조회해서 장소 목록 필터링과 북마크 Set에 재사용 // (기존에는 getBookmarkedPlacesByLatest()와 북마크 Set 생성 시 각각 1회씩, 총 2회 호출했음) List<Long> bookmarkedOrderedIds = (isOnlyBookmarkSearch && userId != null) ? placeBookmarkFacade.getBookmarkedPlaceIdsForTown(userId, townId) : null; @@ // 북마크 검색 시 orderedIds를 함께 전달 → 내부에서 북마크 최신순으로 재정렬 List<Place> places = getPlacesByCondition(townId, isOnlyBookmarkSearch, mainTagId, subTagAIdList, subTagBIdList, bookmarkedOrderedIds); + + if (places.isEmpty()) { + return PlaceFilterGetResponse.from(List.of()); + }이렇게 올리면 아래
getPlacesByCondition()의 동일 검증은 함께 제거해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/solply_server/domain/place/service/PlaceService.java` around lines 113 - 134, The code currently calls placeBookmarkFacade.getBookmarkedPlaceIdsForTown(...) before validating the tag combination inside getPlacesByCondition, causing unnecessary Redis/DB hits; change the flow so you first validate/tag-filter via getPlacesByCondition (or extract its validation logic into a pre-check) and only call placeBookmarkFacade.getBookmarkedPlaceIdsForTown when isOnlyBookmarkSearch is true AND the tag validation passes (or when places is non-empty for non-bookmark logged-in flows), then reuse that ordered list to build bookmarkedIds; also remove the now-duplicated tag validation from getPlacesByCondition so no double-check occurs (referencing isOnlyBookmarkSearch, bookmarkedOrderedIds, getPlacesByCondition, getBookmarkedPlaceIdsForTown, and bookmarkedIds).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/resources/db/migration/V10__optimize_indexes_for_bookmark_query.sql`:
- Around line 3-5: The migration drops idx_bookmark_user_created before creating
the new idx_bookmark_user_type_created which leaves bookmarks without the main
index during migration; change the migration to create the new index on
bookmarks (CREATE INDEX [CONCURRENTLY] idx_bookmark_user_type_created ON
bookmarks (user_id, target_type, created_at DESC)) first, wait until it is
ready, and only then drop the old idx_bookmark_user_created (DROP INDEX
[CONCURRENTLY] idx_bookmark_user_created) to avoid a window with no index and
minimize locking.
In `@src/main/resources/spy.properties`:
- Around line 1-10: spy.properties is loaded in all profiles and P6Spy is on the
main classpath (see build.gradle), causing SQL logging in production; fix by
choosing one of: move spy.properties into a profile-specific resource directory
(e.g., src/main/resources-<profile>/spy.properties) so only that profile enables
P6Spy, or add spring.autoconfigure.exclude for the P6Spy auto-configuration in
application-prod.yml to disable it in production, or change the P6Spy dependency
scope in build.gradle to testOnly so it is not on the main classpath; update the
relevant artifact (spy.properties, build.gradle, or application-prod.yml)
accordingly and verify production profile does not load P6Spy.
---
Nitpick comments:
In `@src/main/java/org/sopt/solply_server/domain/place/service/PlaceService.java`:
- Around line 113-134: The code currently calls
placeBookmarkFacade.getBookmarkedPlaceIdsForTown(...) before validating the tag
combination inside getPlacesByCondition, causing unnecessary Redis/DB hits;
change the flow so you first validate/tag-filter via getPlacesByCondition (or
extract its validation logic into a pre-check) and only call
placeBookmarkFacade.getBookmarkedPlaceIdsForTown when isOnlyBookmarkSearch is
true AND the tag validation passes (or when places is non-empty for non-bookmark
logged-in flows), then reuse that ordered list to build bookmarkedIds; also
remove the now-duplicated tag validation from getPlacesByCondition so no
double-check occurs (referencing isOnlyBookmarkSearch, bookmarkedOrderedIds,
getPlacesByCondition, getBookmarkedPlaceIdsForTown, and bookmarkedIds).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: e856d228-94e4-4d2a-9388-868a271da4ce
📒 Files selected for processing (8)
.gitignorebuild.gradlesrc/main/java/org/sopt/solply_server/domain/bookmark/entity/Bookmark.javasrc/main/java/org/sopt/solply_server/domain/place/entity/Place.javasrc/main/java/org/sopt/solply_server/domain/place/repository/querydsl/PlaceRepositoryImpl.javasrc/main/java/org/sopt/solply_server/domain/place/service/PlaceService.javasrc/main/resources/db/migration/V10__optimize_indexes_for_bookmark_query.sqlsrc/main/resources/spy.properties
- $env.TEST_TOKEN 환경변수 → users.csv 토큰 풀로 교체 - order: random으로 요청마다 다른 유저 선택 - load-test/data/ gitignore 추가 (토큰 포함 파일 보호)
uykm
changed the title
- Artillery 리포트·p6spy 쿼리 로그·EXPLAIN 실행계획 분석용 Claude Code 로컬 에이전트 - .claude/agents/perf-analyzer.md 생성
uykm
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/perf-analyzer.md:
- Around line 40-44: The markdown blocks in .claude/agents/perf-analyzer.md are
missing fenced code block language tags causing markdownlint MD040 warnings; for
each example block (the "분석 대상" list, the N+1 detection block, the slow query
block, and the EXPLAIN analysis block) add an appropriate language tag (e.g.,
text for plain text examples, sql for queries, markdown where applicable)
immediately after the opening ``` so the fences become ```text or ```sql as
shown in the suggested snippets; update every similar code fence referenced
(around the sections you noted: lines ~40, 108-114, 120-124, 151-161) to remove
the CI warning.
- Around line 76-80: The KPI thresholds under "KPI 임계값" are overlapping for p95
(p95 > 3,000ms and p95 > 1,000ms) and must be made mutually exclusive or
prioritized so only one severity is emitted; update the p95 rules in the "KPI
임계값" block to use exclusive ranges (e.g., p95 > 3000ms → [심각], 1000ms < p95 ≤
3000ms → [경고], p95 ≤ 1000ms → [주의]) or explicitly document a priority order
(e.g., check >3000ms first, then >1000ms) so the analyzer logic tied to these
labels produces a single severity per measurement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 05a1b65c-8875-471a-83bf-818b2571e6e5
📒 Files selected for processing (5)
.claude/agents/perf-analyzer.md.claude/settings.local.jsonbuild.gradlesrc/main/resources/spy.propertiessrc/test/resources/application-test.yml
✅ Files skipped from review due to trivial changes (3)
- src/test/resources/application-test.yml
- .claude/settings.local.json
- src/main/resources/spy.properties
🚧 Files skipped from review as they are similar to previous changes (1)
- build.gradle
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.claude/agents/perf-analyzer.md (1)
40-44:⚠️ Potential issue | 🟡 Minor펜스 코드블록 언어 태그를 지정해 MD040 경고를 제거해주세요.
Line 40, Line 151의 코드블록 시작 펜스에 언어가 없어 markdownlint 경고가 발생합니다. 두 블록 모두
text로 지정하면 됩니다.제안 수정안
-``` +```text 분석 대상: - 리포트: place/01~04 (4개), course/01,02,04 (3개) - 쿼리 로그: place-list-bookmark/01,03,04 (3개), folder-preview/01,03,04 (3개)-
+text
EXPLAIN 분석 결과
테이블: bookmarks
type: ALL → [심각] Full Table Scan 감지
key: NULL → 인덱스 미사용
rows: 50,000문제: user_id + target_type 복합 인덱스 없음
권장: INDEX(user_id, target_type, target_id) 추가
예상 효과: rows 50,000 → ~10 (사용자당 평균 북마크 수 기준)Also applies to: 151-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/perf-analyzer.md around lines 40 - 44, Two fenced code blocks in .claude/agents/perf-analyzer.md are missing a language tag (causing MD040); update the opening backticks for both blocks (the one around "분석 대상:" and the one around "EXPLAIN 분석 결과") to include the language tag "text" (i.e., change ``` to ```text) so markdownlint no longer warns; ensure both occurrences (the blocks at the earlier section and the later EXPLAIN section) are updated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.claude/agents/perf-analyzer.md:
- Around line 40-44: Two fenced code blocks in .claude/agents/perf-analyzer.md
are missing a language tag (causing MD040); update the opening backticks for
both blocks (the one around "분석 대상:" and the one around "EXPLAIN 분석 결과") to
include the language tag "text" (i.e., change ``` to ```text) so markdownlint no
longer warns; ensure both occurrences (the blocks at the earlier section and the
later EXPLAIN section) are updated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 5a57b8af-cfd6-4fa6-a338-357ec0a1efc1
📒 Files selected for processing (1)
.claude/agents/perf-analyzer.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개요
Artillery 부하테스트(최대 50 RPS)에서 아래 API들이 DB 커넥션 풀을 소진하는 문제를 발견하고 원인을 제거했습니다.
GET /api/places/bookmarks/folders/previewGET /api/places?isBookmarkSearch=trueGET /api/courses/bookmarks/folders/previewGET /api/courses/bookmarks원인 분석
1. N+1 쿼리 (장소 API)
PlaceRepositoryImpl에서 장소 목록 조회 후loadPlaceTagsAndTags()를 별도 호출하는 구조였으나, 이 메서드는Place.placeTags컬렉션을 초기화하지 못했습니다. 결과적으로 이후place.getMainTag()호출 시 장소 수만큼 추가 쿼리가 발생(N+1)하여 50 RPS 기준 성공률이 15.8%까지 떨어졌습니다.2. 중복 Redis 호출 (장소 API)
PlaceService.getPlacesByTownAndTag()에서getBookmarkedPlaceIdsForTown()을 2회 호출하는 구조.3. 인덱스 불일치 + 콜드 스타트 동시 백필 (코스 API)
bookmarks(user_id, created_at)WHERE user_id=? AND target_type=?target_type누락 → user_id 매칭 전체 행을 메모리 필터링places(town_id)WHERE town_id=? AND active=trueactive누락 → town_id 매칭 전체 행을 메모리 필터링코스 API는 Redis ZSET 캐시를 사용하므로 정상 상태에서는 캐시 히트로 DB 조회가 발생하지 않습니다. 그러나 캐시 콜드 스타트 시
getLatestBookmarkedCourseIdPerTown()에서 동기화 없이 towns-set 미스를 감지한 모든 동시 요청이fullBackfillCourses()를 동시에 실행합니다(single-flight 패턴 없음).기존 인덱스로는 이 백필 쿼리(
findAllCourseBookmarksWithTownId)가 느려 커넥션을 오래 점유했고, 50 RPS 부하에서 콜드 스타트 구간에 동시 백필이 몰리면서 커넥션 풀이 고갈되었습니다. 인덱스 최적화 후 백필 쿼리가 빠르게 완료되면서 이 구간의 영향이 제거되었습니다.변경 사항
Step 1 — 쿼리 최적화
PlaceRepositoryImplfindPlacesWithoutTags()/findPlacesWithTags():loadPlaceTagsAndTags()제거 →LEFT JOIN FETCH로 1쿼리 통합, N+1 완전 제거loadPlaceTagsAndTags()메서드 삭제PlaceServicegetBookmarkedPlaceIdsForTown()상단 1회 호출 후 재사용 (중복 호출 제거)isBookmarkSearch=true일 때 조회된 orderedIds를 북마크 최신순 정렬에도 활용Step 2 — 인덱스 최적화 (Flyway V10, V11)
Step 3 — 부하테스트 시나리오 개선
$env.TEST_TOKEN단일 유저 방식 →users.csv토큰 풀 +order: random으로 전환load-test/data/gitignore 추가 (토큰 포함 파일 보호)성능 측정 결과
EXPLAIN 실행 계획 비교
places테이블 —WHERE town_id=? AND active=true(town_id)(town_id, active)풀 테이블 스캔 → 인덱스 탐색으로 개선.
active필터링이 인덱스 내에서 처리되어 스캔 행 수가 320 → 59로 감소합니다. 해당 쿼리는 LIMIT 없이 전체 행을 조회하므로 filesort는 애초에 유의미한 오버헤드가 아니며,created_at은 인덱스에서 제외했습니다.bookmarks테이블 — 유니크 제약이 커버링 인덱스로 동작하는 두 쿼리uk_bookmark_user_target UNIQUE (user_id, target_type, target_id)는 단순 중복 방지용 제약이 아닌, 아래 두 쿼리에서 커버링 인덱스로 동작합니다. InnoDB에서 UNIQUE 제약은 B-Tree 인덱스로 구현되므로, 쿼리 실행에 필요한 모든 컬럼이 인덱스에 포함될 경우 테이블 heap 접근이 발생하지 않습니다.① EXISTS 체크 —
existsByUserIdAndTargetTypeAndTargetIdSELECT 절이 상수(
1)이므로 WHERE 조건 컬럼만 인덱스에 있으면 충분합니다.(user_id, target_type, target_id)세 컬럼이 모두 UK에 포함되어 테이블 접근 없이 인덱스만으로 존재 여부를 판정합니다.② backfill 조회 —
findBookmarkedTargetIdsByTargetIdsSELECT 절의
target_id도 인덱스에 포함되어 있어 테이블 heap 접근이 0회입니다.IN (...)조건은(user_id, target_type)등치로 범위를 먼저 좁힌 뒤target_id를 인덱스 내에서만 필터링하므로, range scan이더라도 추가 I/O가 발생하지 않습니다.부하테스트 (Artillery 50 RPS)
장소 API
코스 API (캐시 콜드 스타트 기준)
코스 API의 극적인 개선은 콜드 스타트 시 동시 백필 쿼리가 새 인덱스를 타면서 빠르게 완료된 결과입니다.
체크리스트
./gradlew build -x test)@Index어노테이션 동기화Closes #352